CheckboxControl: move icons out of labels #45535
Conversation
|
|
mirka
added
[Package] Block editor
| margin-right: $grid-unit-15; | ||
| fill: $gray-900; | ||
| } | ||
| svg { |
There was a problem hiding this comment.
We need to be careful with element selectors like svg, as in this case where it's unintentionally applying to the checkmark svg as well 😄
The normal suggested way would be to add our own className to Icon for better scoping.
It might also be nice to add a flex-grow: 0 in here so the icon doesn't shrink when the checkbox text is long.
There was a problem hiding this comment.
We need to be careful with element selectors like svg, as in this case where it's unintentionally applying to the checkmark svg as well 😄
Good catch! I usually have a good eye for that, so I'm surprised I missed that! 🤦♀️
It might also be nice to add a
flex-grow: 0in here so the icon doesn't shrink when the checkbox text is long.
I might be missing something, but I'm having trouble getting flex-grow: 0 to work in that scenario. Would it be a bad idea to set a min-width on the icon instead?
There was a problem hiding this comment.
I think I meant flex-shrink 🙈 Does that work?
| } | ||
| svg { | ||
| margin-right: $grid-unit-15; | ||
| fill: $gray-900; |
There was a problem hiding this comment.
The fill hack is not ideal (#40102), but I understand why it's needed at the moment. (No action required, just mentioning for future context.)
| @@ -25,24 +25,14 @@ | |||
| } | |||
| } | |||
| .block-editor-block-lock-modal__checklist-item { | |||
| display: flex; | |||
| justify-content: space-between; | |||
There was a problem hiding this comment.
Would it be worth adding a flex-gap so the text and icon don't run together when the text is long?
There was a problem hiding this comment.
That makes sense to me!
brookewp
force-pushed
the
checkboxcontrol/move-label-icons
branch
from
8c4891e to
9285615
Compare
brookewp
force-pushed
the
checkboxcontrol/move-label-icons
branch
from
9285615 to
bfef1c6
Compare
What?
The icons for
BlockLockModalandBlockTypesChecklistwere nested inside theCheckboxControllabel. This PR moves them out of the label and removes unnecessary CSS.Why?
This PR is based on the comment here: #45434 (comment) by @mirka
How?
Moving the
IconandBlockIconcomponents out of thelabelprop. Also, removing the CSS forcomponents-base-control__fieldandcomponents-checkbox-control__labeland moving it to the parent class for the respective components.Testing Instructions
For
BlockLockModalRestrict editingthat some other blocks don't have, i.e the Paragraph block)-- Make sure the icon shows the correct lock based on if it's locked or unlocked
-- Ensure background hover works as expected
-- Changes are responsive
For
BlockTypesChecklist-- Ensure it looks the same as before (it's about a pixel different in vertical spacing, not sure if that'll be an issue)
-- Check that it's responsive